Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release 2.0 #19

Open
wants to merge 39 commits into
base: main
Choose a base branch
from
Open

Release 2.0 #19

wants to merge 39 commits into from

Conversation

Prashant-7718
Copy link
Collaborator

  1. Local code edit and preview
  2. Panel control options : zoom in out, pan
  3. Syntax highlighting
  4. vscode theme for mermaid light and dark
  5. Commands: Logout, login
  6. Right click menu option : Preview Diagram
  7. Global settings for theme
  8. code lenses integration with Aux files : .htm, md, .hugo
  9. Automatic connect to mermaid chart
  10. key binding for cnt + s or cmd + s for saving diagram to mermaid chart and local

Comment on lines +67 to +70
"aliases": [
"Mermaid Flowchart",
"mermaid flowchart"
],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need the aliases in both cases?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it is not really needed but it is good to keep to avoid edge cases where Other VSCode extensions might refer to the language using different casing.

package.json Outdated
Comment on lines 492 to 498
"command": "mermaidChart.editMermaidChart",
"title": "Edit Diagram in Mermaid chart"
},
{
"command": "mermaidChart.editLocally",
"title": "Edit Diagram"
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't these two need the MermaidChart: prefix?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, these commands are not exposed to users, so they do not need the MermaidChart: prefix. Instead, I have removed them from package.json

"vite": "^6.0.7"
},
"dependencies": {
"@mermaid-chart/mermaid": "11.4.1-b.2",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this package is public.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it is not public and because of this we decided to make vs code plugin repository private.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please setup PNPM workspace properly, there should only be one pnpm-lock.yaml file.

Comment on lines 309 to 324
.icon:hover {
box-shadow: 0px 0px 4px var(--shadow-color);
background-color: var(--shadow-color);
}
svg {
width: 25px;
height: 25px;
transition: stroke 0.3s, fill 0.3s;
}
.icon.active {
background-color: var(--shadow-color);
box-shadow: 0px 0px 4px var(--shadow-color);
}

.icon span {
width: 20px;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please run prettier on all the files (or setup properly in the repo)

}

const document = activeEditor?.document;
if (document && document?.languageId !== "plaintext" && !document.fileName.endsWith(".mmd") && !document.fileName.endsWith(".mermaid") && !document.languageId.startsWith('mermaid')) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

document? is not required if document && is already checked.

)
vscode.workspace.onWillSaveTextDocument(async (event) => {
if (event.document.languageId.startsWith("mermaid")) {
event.waitUntil(Promise.resolve([]));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this required?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The onWillSaveTextDocument event is used to sync changes from the local file to Mermaid Chart. The event.waitUntil(Promise.resolve([])) prevents unintended modifications from other extensions (such as formatters or linters), ensuring that the file remains unchanged during the save process.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does it do that, can you share some docs for that, and add a comment there?

Comment on lines +234 to +246
public async saveDocumentCode(code: string, documentID: string): Promise<MCDocument> {
const response = await this.axios.patch(this.URLS.rest.documents.patch(documentID), {
code: code,
});
return response.data;
}
public async createDocumentWithDiagram(code: string, projectID: string): Promise<MCDocument> {
const response = await this.axios.post(this.URLS.rest.projects.get(projectID).documents, {
code : code
});
return response.data;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the mermaidchart SDK instead of custom code.

https://github.com/Mermaid-Chart/plugins/tree/main/packages/sdk

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, @sidharthv96. Since the custom API code is already in place, can we create a separate ticket to migrate to the MermaidChart SDK later based on our current priorities?

import * as fs from 'fs';
import * as vscode from 'vscode';

export function getFirstWord(text: string): string {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See https://github.com/mermaid-js/mermaid/blob/c0187101e9cb3ea9d1966575efcd80ebb6a9a203/packages/mermaid/src/preprocess.ts on how we handle this inside mermaid, the same approach is preferred, instead of custom parsing logic

<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>Svelte Mermaid Preview</title>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<title>Svelte Mermaid Preview</title>
<title>Mermaid Preview</title>

Comment on lines +75 to +80
"architecture": [
"architecture"
],
"packet": [
"packet"
],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't these in beta as well? Double check all the diagrams for beta.

activeEditor.document,
comments
);
let mermaidChartTokens
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add type to this.

})
);
const syncFileToMermaidChart = async (document: vscode.TextDocument) => {
if (document.languageId.startsWith("mermaid")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Early return.

Comment on lines +59 to +68
export function extractIdFromCode(code: string): string | null {
const { frontMatter } = splitFrontMatter(code);
if (!frontMatter) return null; // No frontmatter present

const document = parseFrontMatterYAML(frontMatter);
const id = document.contents.get('id');

return typeof id === 'string' ? id : null; // Ensure 'id' is a string
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use undefined instead of null

Comment on lines +5 to +35
const frontMatterRegex = /^-{3}\s*[\n\r](.*?[\n\r])-{3}\s*[\n\r]+/s;
const YAML_BLOCK_REGEX = /^\s*---[\r\n]+([\s\S]+?)[\r\n]+\s*---/gm;
const EMPTY_BLOCK_REGEX = /^\s*---\s*\n\s*---\s*\n?/;
const COMMENT_REGEX = /^\s*%%(?!{)[^\n]+\n?/gm;
const DIRECTIVE_REGEX = /%{2}{\s*(?:(\w+)\s*:|(\w+))\s*(?:(\w+)|((?:(?!}%{2}).|\r?\n)*))?\s*(?:}%{2})?/gi;
const FIRST_WORD_REGEX = /^\s*(\w+)/;

export const anyCommentRegex = /\s*%%.*\n/gm;

function parseFrontMatterYAML(frontMatterYaml: string): Document<YAMLMap, false> {
const document: Document = parseDocument(frontMatterYaml);
if (!isMap(document.contents)) {
document.contents = new YAMLMap();
}
return document as unknown as Document<YAMLMap, false>;
}

function splitFrontMatter(text: string) {
const matches = text.match(frontMatterRegex);
if (!matches || !matches[1]) {
return {
diagramText: text,
frontMatter: '',
};
} else {
return {
diagramText: text.slice(matches[0].length),
frontMatter: matches[1],
};
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check if you can use mermaid's internal functions to parse these, no need to duplicate.


const element = document.getElementById("mermaid-diagram");
if (element && diagramContent) {
if (diagramContent === " ") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we checking for an empty space?

const currentPan = panzoomInstance?.getPan() || { x: 0, y: 0 };
const { svg } = await mermaid.render("diagram-graph", diagramContent);
element.innerHTML = svg;
if (theme && (theme === "dark" || theme === "neo-dark" )) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (theme && (theme === "dark" || theme === "neo-dark" )) {
if (theme?.endsWith("dark")) {

Comment on lines +113 to +115
}

if (isToggled) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
if (isToggled) {
} else {

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed toggle as it is not in use

type: "error",
message: errorMessage,
});
isErrorOccured = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
isErrorOccured = true
hasErrorOccured = true

Comment on lines +1 to +3
<script>
export let fill;
</script>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add type

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants